Skip to content

Commit de8c5dc

Browse files
committed
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.
1 parent f657658 commit de8c5dc

File tree

3 files changed

+20
-13
lines changed

3 files changed

+20
-13
lines changed

fuzz/src/router.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,10 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
236236
}
237237
&last_hops_vec[..]
238238
};
239-
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));
239+
let _ = get_route(&our_pubkey, &net_graph_msg_handler.network_graph.read().unwrap(), &target,
240+
first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
241+
&last_hops.iter().collect::<Vec<_>>(),
242+
slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger));
240243
},
241244
_ => return,
242245
}

lightning/src/ln/functional_tests.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3625,7 +3625,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
36253625
let logger = test_utils::TestLogger::new();
36263626
let payment_event = {
36273627
let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
3628-
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();
3628+
let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(),
3629+
&nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
3630+
&Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap();
36293631
nodes[0].node.send_payment(&route, payment_hash_1, &None).unwrap();
36303632
check_added_monitors!(nodes[0], 1);
36313633

@@ -3800,7 +3802,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
38003802

38013803
// Channel should still work fine...
38023804
let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
3803-
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();
3805+
let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(),
3806+
&nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
3807+
&Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap();
38043808
let payment_preimage_2 = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
38053809
claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
38063810
}

lightning/src/routing/router.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
1515
use bitcoin::secp256k1::key::PublicKey;
1616

17-
use ln::channelmanager;
17+
use ln::channelmanager::ChannelDetails;
1818
use ln::features::{ChannelFeatures, NodeFeatures};
1919
use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
2020
use routing::network_graph::{NetworkGraph, RoutingFees};
@@ -169,8 +169,8 @@ struct DummyDirectionalChannelInfo {
169169
/// The fees on channels from us to next-hops are ignored (as they are assumed to all be
170170
/// equal), however the enabled/disabled bit on such channels as well as the htlc_minimum_msat
171171
/// *is* checked as they may change based on the receiving node.
172-
pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, target: &PublicKey, first_hops: Option<&[channelmanager::ChannelDetails]>,
173-
last_hops: &[RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
172+
pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, target: &PublicKey, first_hops: Option<&[&ChannelDetails]>,
173+
last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
174174
// TODO: Obviously *only* using total fee cost sucks. We should consider weighting by
175175
// uptime/success in using a node in the past.
176176
if *target == *our_node_id {
@@ -907,7 +907,7 @@ mod tests {
907907
inbound_capacity_msat: 0,
908908
is_live: true,
909909
}];
910-
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();
910+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
911911
assert_eq!(route.paths[0].len(), 2);
912912

913913
assert_eq!(route.paths[0][0].pubkey, nodes[7]);
@@ -954,7 +954,7 @@ mod tests {
954954
inbound_capacity_msat: 0,
955955
is_live: true,
956956
}];
957-
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();
957+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
958958
assert_eq!(route.paths[0].len(), 2);
959959

960960
assert_eq!(route.paths[0][0].pubkey, nodes[7]);
@@ -1018,7 +1018,7 @@ mod tests {
10181018
inbound_capacity_msat: 0,
10191019
is_live: true,
10201020
}];
1021-
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();
1021+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
10221022
assert_eq!(route.paths[0].len(), 2);
10231023

10241024
assert_eq!(route.paths[0][0].pubkey, nodes[7]);
@@ -1071,7 +1071,7 @@ mod tests {
10711071
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
10721072

10731073
// Simple test across 2, 3, 5, and 4 via a last_hop channel
1074-
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();
1074+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
10751075
assert_eq!(route.paths[0].len(), 5);
10761076

10771077
assert_eq!(route.paths[0][0].pubkey, nodes[1]);
@@ -1130,7 +1130,7 @@ mod tests {
11301130
is_live: true,
11311131
}];
11321132
let mut last_hops = last_hops(&nodes);
1133-
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();
1133+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
11341134
assert_eq!(route.paths[0].len(), 2);
11351135

11361136
assert_eq!(route.paths[0][0].pubkey, nodes[3]);
@@ -1150,7 +1150,7 @@ mod tests {
11501150
last_hops[0].fees.base_msat = 1000;
11511151

11521152
// Revert to via 6 as the fee on 8 goes up
1153-
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();
1153+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
11541154
assert_eq!(route.paths[0].len(), 4);
11551155

11561156
assert_eq!(route.paths[0][0].pubkey, nodes[1]);
@@ -1184,7 +1184,7 @@ mod tests {
11841184
assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
11851185

11861186
// ...but still use 8 for larger payments as 6 has a variable feerate
1187-
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();
1187+
let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, &last_hops.iter().collect::<Vec<_>>(), 2000, 42, Arc::clone(&logger)).unwrap();
11881188
assert_eq!(route.paths[0].len(), 5);
11891189

11901190
assert_eq!(route.paths[0][0].pubkey, nodes[1]);

0 commit comments

Comments
 (0)