From dcf9b4d4b169af97d7f7f8ad9a370e3650771765 Mon Sep 17 00:00:00 2001 From: elnosh Date: Mon, 26 May 2025 10:50:36 -0500 Subject: [PATCH 1/7] sim_node/feat: track payment path with in flight --- simln-lib/src/sim_node.rs | 44 +++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index 1faee0d3..f1dbf77d 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -489,6 +489,13 @@ pub trait SimNetwork: Send + Sync { type LdkNetworkGraph = NetworkGraph>; +struct InFlightPayment { + /// The channel used to report payment results to. + track_payment_receiver: Receiver>, + /// The path the payment was dispatched on. + path: Path, +} + /// A wrapper struct used to implement the LightningNode trait (can be thought of as "the" lightning node). Passes /// all functionality through to a coordinating simulation network. This implementation contains both the [`SimNetwork`] /// implementation that will allow us to dispatch payments and a read-only NetworkGraph that is used for pathfinding. @@ -498,7 +505,7 @@ pub struct SimNode { /// The underlying execution network that will be responsible for dispatching payments. network: Arc>, /// Tracks the channel that will provide updates for payments by hash. - in_flight: Mutex>>>, + in_flight: Mutex>, /// A read-only graph used for pathfinding. pathfinding_graph: Arc, /// Probabilistic scorer used to rank paths through the network for routing. This is reused across @@ -548,17 +555,18 @@ impl SimNode { let (sender, receiver) = channel(); // Check for payment hash collision, failing the payment if we happen to repeat one. - let mut in_flight = self.in_flight.lock().await; - match in_flight.entry(payment_hash) { + match self.in_flight.lock().await.entry(payment_hash) { Entry::Occupied(_) => { return Err(LightningError::SendPaymentError( "payment hash exists".to_string(), )); }, - Entry::Vacant(vacant) => { - vacant.insert(receiver); - }, - } + Entry::Vacant(vacant) => vacant.insert(InFlightPayment { + track_payment_receiver: receiver, + path: route.paths[0].clone(), // TODO: MPP payments? we check in dispatch_payment + // should probably only pass a single path to dispatch + }), + }; self.network.lock().await.dispatch_payment( self.info.pubkey, @@ -640,16 +648,15 @@ impl LightningNode for SimNode { let payment_hash = preimage.into(); // Check for payment hash collision, failing the payment if we happen to repeat one. - match self.in_flight.lock().await.entry(payment_hash) { + let mut in_flight_guard = self.in_flight.lock().await; + let entry = match in_flight_guard.entry(payment_hash) { Entry::Occupied(_) => { return Err(LightningError::SendPaymentError( "payment hash exists".to_string(), )); }, - Entry::Vacant(vacant) => { - vacant.insert(receiver); - }, - } + Entry::Vacant(vacant) => vacant, + }; // Use the stored scorer when finding a route let route = match find_payment_route( @@ -676,6 +683,12 @@ impl LightningNode for SimNode { }, }; + entry.insert(InFlightPayment { + track_payment_receiver: receiver, + path: route.paths[0].clone(), // TODO: how to handle non-MPP support (where would we do + // paths in the world where we have them?). + }); + // If we did successfully obtain a route, dispatch the payment through the network and then report success. self.network.lock().await.dispatch_payment( self.info.pubkey, @@ -696,9 +709,8 @@ impl LightningNode for SimNode { hash: &PaymentHash, listener: Listener, ) -> Result { - let mut in_flight = self.in_flight.lock().await; - match in_flight.remove(hash) { - Some(receiver) => { + match self.in_flight.lock().await.remove(hash) { + Some(in_flight) => { select! { biased; _ = listener => Err( @@ -706,7 +718,7 @@ impl LightningNode for SimNode { ), // If we get a payment result back, remove from our in flight set of payments and return the result. - res = receiver => { + res = in_flight.track_payment_receiver => { res.map_err(|e| LightningError::TrackPaymentError(format!("channel receive err: {}", e)))? }, } From d974129d9663d3497d4f65d28aeb424e5929128a Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 10 Jun 2025 16:14:37 -0400 Subject: [PATCH 2/7] simln-lib/feat: Add IndexFailure payment outcome --- simln-lib/src/lib.rs | 2 ++ simln-lib/src/sim_node.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/simln-lib/src/lib.rs b/simln-lib/src/lib.rs index 3050495a..79e1dbc1 100755 --- a/simln-lib/src/lib.rs +++ b/simln-lib/src/lib.rs @@ -456,6 +456,8 @@ pub enum PaymentOutcome { NotDispatched, /// The payment was dispatched but its final status could not be determined. TrackPaymentFailed, + /// The payment failed at the provided index in the path. + IndexFailure(usize), } /// Describes a payment from a source node to a destination node. diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index f1dbf77d..c0e49d3b 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -1467,7 +1467,7 @@ async fn propagate_payment(request: PropagatePaymentRequest) { ); PaymentResult { htlc_count: 0, - payment_outcome: PaymentOutcome::Unknown, + payment_outcome: PaymentOutcome::IndexFailure(fail_idx.unwrap_or(0)), } }, Err(critical_err) => { @@ -2536,7 +2536,7 @@ mod tests { assert!(matches!( result.unwrap().payment_outcome, - PaymentOutcome::Unknown + PaymentOutcome::IndexFailure(_) )); // The interceptor returned a forwarding error, check that a simulation shutdown has not From db43e330cfe103feea5a7a085e5d2e1ca75339a0 Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 25 Jun 2025 16:06:25 +0100 Subject: [PATCH 3/7] sim-ln/feat: Track Clock in SimNode --- sim-cli/src/parsing.rs | 4 ++-- simln-lib/src/lib.rs | 3 +++ simln-lib/src/sim_node.rs | 36 +++++++++++++++++++++++------------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/sim-cli/src/parsing.rs b/sim-cli/src/parsing.rs index 32484fd9..523cf0d3 100755 --- a/sim-cli/src/parsing.rs +++ b/sim-cli/src/parsing.rs @@ -262,7 +262,7 @@ pub async fn create_simulation_with_network( ( Simulation, Vec, - HashMap>>>, + HashMap>>>, ), anyhow::Error, > { @@ -313,7 +313,7 @@ pub async fn create_simulation_with_network( // custom actions on the simulated network. For the nodes we'll pass our simulation, cast them // to a dyn trait and exclude any nodes that shouldn't be included in random activity // generation. - let nodes = ln_node_from_graph(simulation_graph.clone(), routing_graph).await; + let nodes = ln_node_from_graph(simulation_graph.clone(), routing_graph, clock.clone()).await?; let mut nodes_dyn: HashMap<_, Arc>> = nodes .iter() .map(|(pk, node)| (*pk, Arc::clone(node) as Arc>)) diff --git a/simln-lib/src/lib.rs b/simln-lib/src/lib.rs index 79e1dbc1..fe036eb8 100755 --- a/simln-lib/src/lib.rs +++ b/simln-lib/src/lib.rs @@ -263,6 +263,9 @@ pub enum LightningError { /// Error that occurred while getting graph. #[error("Get graph error: {0}")] GetGraphError(String), + /// Error that occured when getting clock info. + #[error("SystemTime conversion error: {0}")] + SystemTimeConversionError(#[from] SystemTimeError), } /// Information about a Lightning Network node. diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index c0e49d3b..9926161c 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -500,7 +500,7 @@ struct InFlightPayment { /// all functionality through to a coordinating simulation network. This implementation contains both the [`SimNetwork`] /// implementation that will allow us to dispatch payments and a read-only NetworkGraph that is used for pathfinding. /// While these two could be combined, we re-use the LDK-native struct to allow re-use of their pathfinding logic. -pub struct SimNode { +pub struct SimNode { info: NodeInfo, /// The underlying execution network that will be responsible for dispatching payments. network: Arc>, @@ -511,16 +511,19 @@ pub struct SimNode { /// Probabilistic scorer used to rank paths through the network for routing. This is reused across /// multiple payments to maintain scoring state. scorer: ProbabilisticScorer, Arc>, + /// Clock for tracking simulation time. + clock: Arc, } -impl SimNode { +impl SimNode { /// Creates a new simulation node that refers to the high level network coordinator provided to process payments /// on its behalf. The pathfinding graph is provided separately so that each node can handle its own pathfinding. pub fn new( info: NodeInfo, payment_network: Arc>, pathfinding_graph: Arc, - ) -> Self { + clock: Arc, + ) -> Result { // Initialize the probabilistic scorer with default parameters for learning from payment // history. These parameters control how much successful/failed payments affect routing // scores and how quickly these scores decay over time. @@ -530,13 +533,14 @@ impl SimNode { Arc::new(WrappedLog {}), ); - SimNode { + Ok(SimNode { info, network: payment_network, in_flight: Mutex::new(HashMap::new()), pathfinding_graph, scorer, - } + clock, + }) } /// Dispatches a payment to a specified route. If `custom_records` is `Some`, they will be attached to the outgoing @@ -625,7 +629,7 @@ fn find_payment_route( } #[async_trait] -impl LightningNode for SimNode { +impl LightningNode for SimNode { fn get_info(&self) -> &NodeInfo { &self.info } @@ -1031,11 +1035,12 @@ impl SimGraph { } /// Produces a map of node public key to lightning node implementation to be used for simulations. -pub async fn ln_node_from_graph( +pub async fn ln_node_from_graph( graph: Arc>, routing_graph: Arc, -) -> HashMap>>> { - let mut nodes: HashMap>>> = HashMap::new(); + clock: Arc, +) -> Result>>>, LightningError> { + let mut nodes: HashMap>>> = HashMap::new(); for node in graph.lock().await.nodes.iter() { nodes.insert( @@ -1044,11 +1049,12 @@ pub async fn ln_node_from_graph( node.1 .0.clone(), graph.clone(), routing_graph.clone(), - ))), + clock.clone(), + )?)), ); } - nodes + Ok(nodes) } /// Populates a network graph based on the set of simulated channels provided. This function *only* applies channel @@ -1929,7 +1935,9 @@ mod tests { node_info(pk, String::default()), sim_network.clone(), Arc::new(graph), - ); + Arc::new(SystemClock {}), + ) + .unwrap(); // Prime mock to return node info from lookup and assert that we get the pubkey we're expecting. let lookup_pk = channels[3].node_1.policy.pubkey; @@ -2323,7 +2331,9 @@ mod tests { node_info(test_kit.nodes[0], String::default()), Arc::new(Mutex::new(test_kit.graph)), test_kit.routing_graph.clone(), - ); + Arc::new(SystemClock {}), + ) + .unwrap(); let route = build_route_from_hops( &test_kit.nodes[0], From 26c2bf3868dd2920abd31f01ee69c317267120a0 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 9 Jun 2025 15:10:21 -0400 Subject: [PATCH 4/7] simln-lib/feat: report payments to sim_node scorer --- simln-lib/src/lib.rs | 2 +- simln-lib/src/sim_node.rs | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/simln-lib/src/lib.rs b/simln-lib/src/lib.rs index fe036eb8..12a2511c 100755 --- a/simln-lib/src/lib.rs +++ b/simln-lib/src/lib.rs @@ -433,7 +433,7 @@ impl Display for PaymentResult { } /// Represents all possible outcomes of a Lightning Network payment attempt. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub enum PaymentOutcome { /// Payment completed successfully, reaching its intended recipient. Success, diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index 9926161c..0c1d9b71 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -22,7 +22,9 @@ use lightning::ln::msgs::{ use lightning::ln::{PaymentHash, PaymentPreimage}; use lightning::routing::gossip::{NetworkGraph, NodeId}; use lightning::routing::router::{find_route, Path, PaymentParameters, Route, RouteParameters}; -use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringDecayParameters}; +use lightning::routing::scoring::{ + ProbabilisticScorer, ProbabilisticScoringDecayParameters, ScoreUpdate, +}; use lightning::routing::utxo::{UtxoLookup, UtxoResult}; use lightning::util::logger::{Level, Logger, Record}; use thiserror::Error; @@ -510,7 +512,7 @@ pub struct SimNode { pathfinding_graph: Arc, /// Probabilistic scorer used to rank paths through the network for routing. This is reused across /// multiple payments to maintain scoring state. - scorer: ProbabilisticScorer, Arc>, + scorer: Mutex, Arc>>, /// Clock for tracking simulation time. clock: Arc, } @@ -538,7 +540,7 @@ impl SimNode { network: payment_network, in_flight: Mutex::new(HashMap::new()), pathfinding_graph, - scorer, + scorer: Mutex::new(scorer), clock, }) } @@ -663,12 +665,13 @@ impl LightningNode for SimNode { }; // Use the stored scorer when finding a route + let scorer_guard = self.scorer.lock().await; let route = match find_payment_route( &self.info.pubkey, dest, amount_msat, &self.pathfinding_graph, - &self.scorer, + &scorer_guard, ) { Ok(path) => path, // In the case that we can't find a route for the payment, we still report a successful payment *api call* @@ -723,7 +726,16 @@ impl LightningNode for SimNode { // If we get a payment result back, remove from our in flight set of payments and return the result. res = in_flight.track_payment_receiver => { - res.map_err(|e| LightningError::TrackPaymentError(format!("channel receive err: {}", e)))? + let track_result = res.map_err(|e| LightningError::TrackPaymentError(format!("channel receive err: {}", e)))?; + if let Ok(ref payment_result) = track_result { + let duration = self.clock.now().duration_since(UNIX_EPOCH)?; + if payment_result.payment_outcome == PaymentOutcome::Success { + self.scorer.lock().await.payment_path_successful(&in_flight.path, duration); + } else if let PaymentOutcome::IndexFailure(index) = payment_result.payment_outcome { + self.scorer.lock().await.payment_path_failed(&in_flight.path, index.try_into().unwrap(), duration); + } + } + track_result }, } }, From c1a688d22c27441414871e3048ead9c299176377 Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Mon, 7 Jul 2025 13:15:57 +0100 Subject: [PATCH 5/7] sim_node/feat: Reject MPP Payments This commit ensures that the payment apis can only send to a single path. Payment is rejected if the route has multiple paths. --- simln-lib/src/sim_node.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index 0c1d9b71..2fc88748 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -552,6 +552,8 @@ impl SimNode { /// The [`lightning::routing::router::build_route_from_hops`] function can be used to build the route to be passed here. /// /// **Note:** The payment hash passed in here should be used in track_payment to track the payment outcome. + /// + /// **Note:** The route passed in here must contain only one path. pub async fn send_to_route( &mut self, route: Route, @@ -560,6 +562,12 @@ impl SimNode { ) -> Result<(), LightningError> { let (sender, receiver) = channel(); + if route.paths.len() != 1 { + return Err(LightningError::SendPaymentError( + "Route must contain exactly one path for this operation.".to_string(), + )); + } + // Check for payment hash collision, failing the payment if we happen to repeat one. match self.in_flight.lock().await.entry(payment_hash) { Entry::Occupied(_) => { @@ -690,6 +698,12 @@ impl LightningNode for SimNode { }, }; + if route.paths.len() != 1 { + return Err(LightningError::SendPaymentError( + "Route must contain exactly one path for this operation.".to_string(), + )); + } + entry.insert(InFlightPayment { track_payment_receiver: receiver, path: route.paths[0].clone(), // TODO: how to handle non-MPP support (where would we do @@ -1147,7 +1161,12 @@ impl SimNetwork for SimGraph { payment_hash: PaymentHash, sender: Sender>, ) { - // Expect at least one path (right now), with the intention to support multiple in future. + // Expect only one path (right now), with the intention to support multiple in future. + if route.paths.len() != 1 { + log::error!("Route must contain exactly one path for this operation."); + return; + } + let path = match route.paths.first() { Some(p) => p, None => { From 34f19735b03f73e6f41dbcac0e9ee3a449892708 Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Mon, 7 Jul 2025 20:02:30 +0100 Subject: [PATCH 6/7] sim_node/feat: Test propagate_payment failure on Some(0) This commit asserts that payment failure on first hop returns Some(0). On payment failure, this ensures the correct index of hop where failure occured is reported to scorer. --- simln-lib/src/sim_node.rs | 57 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index 2fc88748..2c89167d 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -2041,6 +2041,63 @@ mod tests { )); } + #[tokio::test] + async fn test_payment_fails_on_first_hop_and_returns_some_0() { + let chan_capacity = 500_000_000; + let test_kit = + DispatchPaymentTestKit::new(chan_capacity, vec![], CustomRecords::default()).await; + + let mut node = SimNode::new( + node_info(test_kit.nodes[0], String::default()), + Arc::new(Mutex::new(test_kit.graph)), + test_kit.routing_graph.clone(), + Arc::new(SystemClock {}), + ) + .unwrap(); + + let mut route = build_route_from_hops( + &test_kit.nodes[0], + &[test_kit.nodes[1], test_kit.nodes[2], test_kit.nodes[3]], + &RouteParameters { + payment_params: PaymentParameters::from_node_id(*test_kit.nodes.last().unwrap(), 0) + .with_max_total_cltv_expiry_delta(u32::MAX) + .with_max_path_count(1) + .with_max_channel_saturation_power_of_half(1), + final_value_msat: 20_000, + max_total_routing_fee_msat: None, + }, + &test_kit.routing_graph, + &WrappedLog {}, + &[0; 32], + ) + .unwrap(); + + // Alter route to make payment fail on first hop (Index 0). + route.paths[0].hops[0].cltv_expiry_delta = 39; + + let preimage = PaymentPreimage(rand::random()); + let payment_hash = preimage.into(); + let send_result = node.send_to_route(route, payment_hash, None).await; + + assert!(send_result.is_ok()); + + let (_, shutdown_listener) = triggered::trigger(); + let result = node + .track_payment(&payment_hash, shutdown_listener) + .await + .unwrap(); + + match result.payment_outcome { + PaymentOutcome::IndexFailure(_) => { + assert_eq!(result.payment_outcome, PaymentOutcome::IndexFailure(0)); + }, + _ => panic!( + "Expected IndexFailure, but got: {:?}", + result.payment_outcome + ), + } + } + mock! { #[derive(Debug)] TestInterceptor{} From 3dfc98a6883af5d844b7951e5a2c88e0ed22f8b2 Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Mon, 7 Jul 2025 20:16:32 +0100 Subject: [PATCH 7/7] Fix formatting issues --- simln-lib/src/sim_node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simln-lib/src/sim_node.rs b/simln-lib/src/sim_node.rs index 2c89167d..4c66b814 100755 --- a/simln-lib/src/sim_node.rs +++ b/simln-lib/src/sim_node.rs @@ -552,7 +552,7 @@ impl SimNode { /// The [`lightning::routing::router::build_route_from_hops`] function can be used to build the route to be passed here. /// /// **Note:** The payment hash passed in here should be used in track_payment to track the payment outcome. - /// + /// /// **Note:** The route passed in here must contain only one path. pub async fn send_to_route( &mut self,