Have `find_route` take a `NetworkGraph` instead of a `ReadOnly` one

Because downstream languages are often garbage-collected, having
the user directly allocate a `ReadOnlyNetworkGraph` and pass a
reference to it to `find_route` often results in holding a read
lock long in excess of the `find_route` call. Worse, some languages
(like JavaScript) tend to only garbage collect when other code is
not running, possibly leading to deadlocks.

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index cff00669a9d6a0dde486987a1ea3124a165e9fdb..b4ca316ed0dbba1f695a913e747239dca3ffef90 100644 (file)
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -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.read_only(), None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                               let route = match find_route(&our_id, &params, &network_graph, 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.read_only(), None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                               let mut route = match find_route(&our_id, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
                                        Ok(route) => route,
                                        Err(_) => return,
                                };

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index cccc7c4f5a7f6cd21eeac3d10560f3d8f2fae298..5dc158053b7bdfa4a10e3a465314223b542180cf 100644 (file)
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -268,7 +268,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.read_only(),
+                                       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);
                                }

diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs
index e09fd836224b80a339564b94d2239bca29ec9347..9ef2d2c8d39214e5a94423fa0c3e98006da9051c 100644 (file)
--- a/lightning-invoice/src/utils.rs
+++ b/lightning-invoice/src/utils.rs
@@ -461,13 +461,12 @@ where L::Target: Logger {
                &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, &network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
+               find_route(payer, params, &self.network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
        }
 }
 
@@ -572,7 +571,7 @@ mod test {
                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.read_only(),
+                       &nodes[0].node.get_our_node_id(), &route_params, &network_graph,
                        Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
                ).unwrap();
 
@@ -848,7 +847,7 @@ mod test {
                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.read_only(),
+                       &nodes[0].node.get_our_node_id(), &params, &network_graph,
                        Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
                ).unwrap();
                let (payment_event, fwd_idx) = {

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 1e3d8de379ac30841922db96e652044a6ca21e32..871ebb92d32270d1d1730dc1c947f1015d2596b8 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7460,7 +7460,7 @@ mod tests {
                        final_cltv_expiry_delta: TEST_FINAL_CLTV,
                };
                let route = find_route(
-                       &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(),
+                       &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
                nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
@@ -7491,7 +7491,7 @@ mod tests {
                // To start (2), send a keysend payment but don't claim it.
                let payment_preimage = PaymentPreimage([42; 32]);
                let route = find_route(
-                       &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(),
+                       &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
                let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
@@ -7555,9 +7555,8 @@ mod tests {
                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(
-                       &payer_pubkey, &route_params, &network_graph.read_only(),
-                       Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer,
-                       &random_seed_bytes
+                       &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+                       nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);
@@ -7600,9 +7599,8 @@ mod tests {
                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(
-                       &payer_pubkey, &route_params, &network_graph.read_only(),
-                       Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer,
-                       &random_seed_bytes
+                       &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+                       nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index a598e7de70d0212122b0c4c42a3754eb6f1b74eb..c304161947af34c31ceed9d28eadad51148191f6 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -9874,7 +9874,7 @@ fn test_keysend_payments_to_public_node() {
        };
        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(&payer_pubkey, &route_params, &network_graph.read_only(), 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)).unwrap();
@@ -9910,8 +9910,8 @@ fn test_keysend_payments_to_private_node() {
        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(
-               &payer_pubkey, &route_params, &network_graph.read_only(),
-               Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer, &random_seed_bytes
+               &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+               nodes[0].logger, &scorer, &random_seed_bytes
        ).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);

diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 26e00319fbdb3b4f2e45fab8d35f51fea576773f..420a473ff53a6228a6da7a0781df9b272ec5a1c6 100644 (file)
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -17,7 +17,7 @@ use bitcoin::secp256k1::PublicKey;
 use ln::channelmanager::ChannelDetails;
 use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NodeId, RoutingFees};
+use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use routing::scoring::{ChannelUsage, Score};
 use util::ser::{Writeable, Readable, Writer};
 use util::logger::{Level, Logger};
@@ -690,16 +690,17 @@ fn default_node_features() -> NodeFeatures {
 /// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels
 /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
 /// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
-pub fn find_route<L: Deref, S: Score>(
+pub fn find_route<L: Deref, GL: Deref, S: Score>(
        our_node_pubkey: &PublicKey, route_params: &RouteParameters,
-       network_graph: &ReadOnlyNetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L,
+       network_graph: &NetworkGraph<GL>, first_hops: Option<&[&ChannelDetails]>, logger: L,
        scorer: &S, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
-       let mut route = get_route(our_node_pubkey, &route_params.payment_params, network_graph, first_hops,
+where L::Target: Logger, GL::Target: Logger {
+       let graph_lock = network_graph.read_only();
+       let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops,
                route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, scorer,
                random_seed_bytes)?;
-       add_random_cltv_offset(&mut route, &route_params.payment_params, network_graph, random_seed_bytes);
+       add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
        Ok(route)
 }
 
@@ -1803,15 +1804,16 @@ fn add_random_cltv_offset(route: &mut Route, payment_params: &PaymentParameters,
 /// exclude the payer, but include the payee). This may be useful, e.g., for probing the chosen path.
 ///
 /// Re-uses logic from `find_route`, so the restrictions described there also apply here.
-pub fn build_route_from_hops<L: Deref>(
+pub fn build_route_from_hops<L: Deref, GL: Deref>(
        our_node_pubkey: &PublicKey, hops: &[PublicKey], route_params: &RouteParameters,
-       network_graph: &ReadOnlyNetworkGraph, logger: L, random_seed_bytes: &[u8; 32]
+       network_graph: &NetworkGraph<GL>, logger: L, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
+where L::Target: Logger, GL::Target: Logger {
+       let graph_lock = network_graph.read_only();
        let mut route = build_route_from_hops_internal(
-               our_node_pubkey, hops, &route_params.payment_params, &network_graph,
+               our_node_pubkey, hops, &route_params.payment_params, &graph_lock,
                route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, random_seed_bytes)?;
-       add_random_cltv_offset(&mut route, &route_params.payment_params, &network_graph, random_seed_bytes);
+       add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
        Ok(route)
 }
 

diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs
index 77b76a0825b1a467278397c43a86ae604b086440..524f0ed3158b40dabae37d9516009a2588b6cd08 100644 (file)
--- a/lightning/src/routing/scoring.rs
+++ b/lightning/src/routing/scoring.rs
@@ -43,7 +43,7 @@
 //! let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
 //! # let random_seed_bytes = [42u8; 32];
 //!
-//! let route = find_route(&payer, &route_params, &network_graph.read_only(), None, &logger, &scorer, &random_seed_bytes);
+//! let route = find_route(&payer, &route_params, &network_graph, None, &logger, &scorer, &random_seed_bytes);
 //! # }
 //! ```
 //!