Skip to content

Commit e0e553a

Browse files
valentinewallacevalentinewallace
committed
OM pathfinding: don't consider nodes that don't support OM forwarding
Or that require unknown features
1 parent 88121b8 commit e0e553a

File tree

3 files changed

+76
-20
lines changed

3 files changed

+76
-20
lines changed

‎lightning/src/routing/onion_message.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ pub fn find_path<L: Deref, GL: Deref>(
6161
return Ok(reverse_path(visited, our_node_id, dest_node_id, logger)?)
6262
}
6363
if let Some(node_info) = network_nodes.get(&node_id) {
64+
if node_id == our_node_id {
65+
} else if let Some(node_ann) = &node_info.announcement_info {
66+
if !node_ann.features.supports_onion_messages() || node_ann.features.requires_unknown_bits()
67+
{ continue; }
68+
}
6469
for scid in &node_info.channels {
6570
if let Some(chan_info) = network_channels.get(&scid) {
6671
if let Some((_, successor)) = chan_info.as_directed_from(&node_id) {
@@ -139,14 +144,17 @@ fn reverse_path<L: Deref>(
139144

140145
#[cfg(test)]
141146
mod tests {
142-
use routing::test_utils;
147+
use ln::features::{InitFeatures, NodeFeatures};
148+
use routing::test_utils::{add_or_update_node, build_graph_with_features, build_line_graph, get_nodes};
143149

144150
use sync::Arc;
145151

146152
#[test]
147153
fn one_hop() {
148-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
149-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
154+
let mut features = NodeFeatures::empty();
155+
features.set_onion_messages_optional();
156+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
157+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
150158

151159
let path = super::find_path(&our_id, &node_pks[7], &network_graph, None, Arc::clone(&logger)).unwrap();
152160
assert_eq!(path.len(), 1);
@@ -155,8 +163,10 @@ mod tests {
155163

156164
#[test]
157165
fn two_hops() {
158-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
159-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
166+
let mut features = NodeFeatures::empty();
167+
features.set_onion_messages_optional();
168+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
169+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
160170

161171
let path = super::find_path(&our_id, &node_pks[2], &network_graph, None, Arc::clone(&logger)).unwrap();
162172
assert_eq!(path.len(), 2);
@@ -167,8 +177,10 @@ mod tests {
167177

168178
#[test]
169179
fn three_hops() {
170-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
171-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
180+
let mut features = NodeFeatures::empty();
181+
features.set_onion_messages_optional();
182+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
183+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
172184

173185
let mut path = super::find_path(&our_id, &node_pks[5], &network_graph, None, Arc::clone(&logger)).unwrap();
174186
assert_eq!(path.len(), 3);
@@ -179,12 +191,34 @@ mod tests {
179191

180192
#[test]
181193
fn long_path() {
182-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_line_graph();
183-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
194+
let mut features = NodeFeatures::empty();
195+
features.set_onion_messages_optional();
196+
let (secp_ctx, network_graph, _, _, logger) = build_line_graph(features);
197+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
184198

185199
let path = super::find_path(&our_id, &node_pks[18], &network_graph, None, Arc::clone(&logger)).unwrap();
186200
assert_eq!(path.len(), 19);
187201
}
202+
203+
#[test]
204+
fn disable_nodes_test() {
205+
// Check that we won't route over nodes that require unknown feature bits.
206+
let mut features = InitFeatures::empty();
207+
features.set_onion_messages_optional();
208+
let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph_with_features(features.to_context());
209+
let (_, our_id, privkeys, node_pks) = get_nodes(&secp_ctx);
210+
211+
// Disable nodes 1, 2, and 8 by requiring unknown feature bits
212+
let mut unknown_features = NodeFeatures::empty();
213+
unknown_features.set_unknown_feature_required();
214+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[0], unknown_features.clone(), 1);
215+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[1], unknown_features.clone(), 1);
216+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[7], unknown_features.clone(), 1);
217+
218+
// If all nodes require some features we don't understand, route should fail
219+
let err = super::find_path(&our_id, &node_pks[2], &network_graph, None, Arc::clone(&logger)).unwrap_err();
220+
assert_eq!(err, super::Error::PathNotFound);
221+
}
188222
}
189223

190224
#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))]

‎lightning/src/routing/router.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5071,7 +5071,8 @@ mod tests {
50715071

50725072
#[test]
50735073
fn limits_path_length() {
5074-
let (secp_ctx, network, _, _, logger) = build_line_graph();
5074+
let features = NodeFeatures::from_le_bytes(id_to_feature_flags(1));
5075+
let (secp_ctx, network, _, _, logger) = build_line_graph(features);
50755076
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
50765077
let network_graph = network.read_only();
50775078

@@ -5349,7 +5350,8 @@ mod tests {
53495350

53505351
#[test]
53515352
fn honors_manual_penalties() {
5352-
let (secp_ctx, network_graph, _, _, logger) = build_line_graph();
5353+
let features = NodeFeatures::from_le_bytes(id_to_feature_flags(1));
5354+
let (secp_ctx, network_graph, _, _, logger) = build_line_graph(features);
53535355
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
53545356

53555357
let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);

‎lightning/src/routing/test_utils.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ pub(super) fn id_to_feature_flags(id: u8) -> Vec<u8> {
167167
}
168168
}
169169

170-
pub(super) fn build_line_graph() -> (
170+
pub(super) fn build_line_graph(node_features: NodeFeatures) -> (
171171
Secp256k1<All>, sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
172172
P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
173173
sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>,
@@ -213,7 +213,7 @@ pub(super) fn build_line_graph() -> (
213213
excess_data: Vec::new()
214214
});
215215
add_or_update_node(&gossip_sync, &secp_ctx, &next_privkey,
216-
NodeFeatures::from_le_bytes(id_to_feature_flags(1)), 0);
216+
node_features.clone(), 0);
217217
}
218218

219219
(secp_ctx, network_graph, gossip_sync, chain_monitor, logger)
@@ -225,6 +225,26 @@ pub(super) fn build_graph() -> (
225225
P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
226226
sync::Arc<test_utils::TestChainSource>,
227227
sync::Arc<test_utils::TestLogger>,
228+
) {
229+
build_graph_inner(None)
230+
}
231+
232+
pub(super) fn build_graph_with_features(features: NodeFeatures) -> (
233+
Secp256k1<All>,
234+
sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
235+
P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
236+
sync::Arc<test_utils::TestChainSource>,
237+
sync::Arc<test_utils::TestLogger>,
238+
) {
239+
build_graph_inner(Some(features))
240+
}
241+
242+
fn build_graph_inner(features: Option<NodeFeatures>) -> (
243+
Secp256k1<All>,
244+
sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
245+
P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
246+
sync::Arc<test_utils::TestChainSource>,
247+
sync::Arc<test_utils::TestLogger>,
228248
) {
229249
let secp_ctx = Secp256k1::new();
230250
let logger = Arc::new(test_utils::TestLogger::new());
@@ -307,7 +327,7 @@ pub(super) fn build_graph() -> (
307327
excess_data: Vec::new()
308328
});
309329

310-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[0], NodeFeatures::from_le_bytes(id_to_feature_flags(1)), 0);
330+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[0], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(1))), 0);
311331

312332
add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(2)), 2);
313333
update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -335,7 +355,7 @@ pub(super) fn build_graph() -> (
335355
excess_data: Vec::new()
336356
});
337357

338-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[1], NodeFeatures::from_le_bytes(id_to_feature_flags(2)), 0);
358+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[1], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(2))), 0);
339359

340360
add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[7], ChannelFeatures::from_le_bytes(id_to_feature_flags(12)), 12);
341361
update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -363,7 +383,7 @@ pub(super) fn build_graph() -> (
363383
excess_data: Vec::new()
364384
});
365385

366-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[7], NodeFeatures::from_le_bytes(id_to_feature_flags(8)), 0);
386+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[7], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(8))), 0);
367387

368388
add_channel(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 3);
369389
update_channel(&gossip_sync, &secp_ctx, &privkeys[0], UnsignedChannelUpdate {
@@ -443,7 +463,7 @@ pub(super) fn build_graph() -> (
443463
excess_data: Vec::new()
444464
});
445465

446-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[2], NodeFeatures::from_le_bytes(id_to_feature_flags(3)), 0);
466+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[2], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(3))), 0);
447467

448468
add_channel(&gossip_sync, &secp_ctx, &privkeys[2], &privkeys[4], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6);
449469
update_channel(&gossip_sync, &secp_ctx, &privkeys[2], UnsignedChannelUpdate {
@@ -497,9 +517,9 @@ pub(super) fn build_graph() -> (
497517
excess_data: Vec::new()
498518
});
499519

500-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[4], NodeFeatures::from_le_bytes(id_to_feature_flags(5)), 0);
520+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[4], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(5))), 0);
501521

502-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[3], NodeFeatures::from_le_bytes(id_to_feature_flags(4)), 0);
522+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[3], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(4))), 0);
503523

504524
add_channel(&gossip_sync, &secp_ctx, &privkeys[2], &privkeys[5], ChannelFeatures::from_le_bytes(id_to_feature_flags(7)), 7);
505525
update_channel(&gossip_sync, &secp_ctx, &privkeys[2], UnsignedChannelUpdate {
@@ -527,7 +547,7 @@ pub(super) fn build_graph() -> (
527547
excess_data: Vec::new()
528548
});
529549

530-
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[5], NodeFeatures::from_le_bytes(id_to_feature_flags(6)), 0);
550+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[5], features.clone().unwrap_or_else(|| NodeFeatures::from_le_bytes(id_to_feature_flags(6))), 0);
531551

532552
(secp_ctx, network_graph, gossip_sync, chain_monitor, logger)
533553
}

0 commit comments

Comments
 (0)