From de8c5dc76dbfc64071763ddf2376b09a9f65479b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 23 Jul 2020 16:10:29 -0400 Subject: [PATCH] Use slices to references not slices of concrete objects in pub API Because the C bindings maps objects into new structs which contain only a pointer to the underlying (immovable) Rust type, it cannot create a list of Rust types which are contiguous in memory. Thus, in order to allow C clients to call certain Rust functions, we have to use &[&Type] not &[Type]. This commit fixes this issue for the get_route function. --- fuzz/src/router.rs | 5 ++++- lightning/src/ln/functional_tests.rs | 8 ++++++-- lightning/src/routing/router.rs | 20 ++++++++++---------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 38e29de51..ffc460837 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -236,7 +236,10 @@ pub fn do_test(data: &[u8], out: Out) { } &last_hops_vec[..] }; - let _ = get_route(&our_pubkey, &net_graph_msg_handler.network_graph.read().unwrap(), &target, first_hops, last_hops, slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger)); + let _ = get_route(&our_pubkey, &net_graph_msg_handler.network_graph.read().unwrap(), &target, + first_hops.map(|c| c.iter().collect::>()).as_ref().map(|a| a.as_slice()), + &last_hops.iter().collect::>(), + slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger)); }, _ => return, } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 7a410c8bf..4b5453351 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3625,7 +3625,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) { let logger = test_utils::TestLogger::new(); let payment_event = { let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; - let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels()), &Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap(); + let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), + &nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels().iter().collect::>()), + &Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap(); nodes[0].node.send_payment(&route, payment_hash_1, &None).unwrap(); check_added_monitors!(nodes[0], 1); @@ -3800,7 +3802,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) { // Channel should still work fine... let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; - let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels()), &Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap(); + let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), + &nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels().iter().collect::>()), + &Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap(); let payment_preimage_2 = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0; claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000); } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 7730fd8f2..23232a0d0 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -14,7 +14,7 @@ use bitcoin::secp256k1::key::PublicKey; -use ln::channelmanager; +use ln::channelmanager::ChannelDetails; use ln::features::{ChannelFeatures, NodeFeatures}; use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT}; use routing::network_graph::{NetworkGraph, RoutingFees}; @@ -169,8 +169,8 @@ struct DummyDirectionalChannelInfo { /// The fees on channels from us to next-hops are ignored (as they are assumed to all be /// equal), however the enabled/disabled bit on such channels as well as the htlc_minimum_msat /// *is* checked as they may change based on the receiving node. -pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, target: &PublicKey, first_hops: Option<&[channelmanager::ChannelDetails]>, - last_hops: &[RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result where L::Target: Logger { +pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, target: &PublicKey, first_hops: Option<&[&ChannelDetails]>, + last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result where L::Target: Logger { // TODO: Obviously *only* using total fee cost sucks. We should consider weighting by // uptime/success in using a node in the past. if *target == *our_node_id { @@ -907,7 +907,7 @@ mod tests { inbound_capacity_msat: 0, is_live: true, }]; - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); assert_eq!(route.paths[0][0].pubkey, nodes[7]); @@ -954,7 +954,7 @@ mod tests { inbound_capacity_msat: 0, is_live: true, }]; - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); assert_eq!(route.paths[0][0].pubkey, nodes[7]); @@ -1018,7 +1018,7 @@ mod tests { inbound_capacity_msat: 0, is_live: true, }]; - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); assert_eq!(route.paths[0][0].pubkey, nodes[7]); @@ -1071,7 +1071,7 @@ mod tests { let (_, our_id, _, nodes) = get_nodes(&secp_ctx); // Simple test across 2, 3, 5, and 4 via a last_hop channel - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops(&nodes), 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops(&nodes).iter().collect::>(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 5); assert_eq!(route.paths[0][0].pubkey, nodes[1]); @@ -1130,7 +1130,7 @@ mod tests { is_live: true, }]; let mut last_hops = last_hops(&nodes); - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], Some(&our_chans), &last_hops, 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], Some(&our_chans.iter().collect::>()), &last_hops.iter().collect::>(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); assert_eq!(route.paths[0][0].pubkey, nodes[3]); @@ -1150,7 +1150,7 @@ mod tests { last_hops[0].fees.base_msat = 1000; // Revert to via 6 as the fee on 8 goes up - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops, 100, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops.iter().collect::>(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 4); assert_eq!(route.paths[0][0].pubkey, nodes[1]); @@ -1184,7 +1184,7 @@ mod tests { assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::::new()); // We can't learn any flags from invoices, sadly // ...but still use 8 for larger payments as 6 has a variable feerate - let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops, 2000, 42, Arc::clone(&logger)).unwrap(); + let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops.iter().collect::>(), 2000, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 5); assert_eq!(route.paths[0][0].pubkey, nodes[1]); -- 2.39.5