Skip to content

Commit d278bb3

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

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
@@ -75,6 +75,11 @@ pub fn find_path<L: Deref, GL: Deref>(
7575
return Ok(path)
7676
}
7777
if let Some(node_info) = network_nodes.get(&node_id) {
78+
if node_id == our_node_id {
79+
} else if let Some(node_ann) = &node_info.announcement_info {
80+
if !node_ann.features.supports_onion_messages() || node_ann.features.requires_unknown_bits()
81+
{ continue; }
82+
}
7883
for scid in &node_info.channels {
7984
if let Some(chan_info) = network_channels.get(&scid) {
8085
if let Some((_, successor)) = chan_info.as_directed_from(&node_id) {
@@ -163,14 +168,17 @@ fn reverse_path(
163168

164169
#[cfg(test)]
165170
mod tests {
166-
use routing::test_utils;
171+
use ln::features::{InitFeatures, NodeFeatures};
172+
use routing::test_utils::{add_or_update_node, build_graph_with_features, build_line_graph, get_nodes};
167173

168174
use sync::Arc;
169175

170176
#[test]
171177
fn one_hop() {
172-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
173-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
178+
let mut features = NodeFeatures::empty();
179+
features.set_onion_messages_optional();
180+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
181+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
174182

175183
let path = super::find_path(&our_id, &node_pks[7], &network_graph, None, Arc::clone(&logger)).unwrap();
176184
assert_eq!(path.len(), 1);
@@ -179,8 +187,10 @@ mod tests {
179187

180188
#[test]
181189
fn two_hops() {
182-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
183-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
190+
let mut features = NodeFeatures::empty();
191+
features.set_onion_messages_optional();
192+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
193+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
184194

185195
let path = super::find_path(&our_id, &node_pks[2], &network_graph, None, Arc::clone(&logger)).unwrap();
186196
assert_eq!(path.len(), 2);
@@ -191,8 +201,10 @@ mod tests {
191201

192202
#[test]
193203
fn three_hops() {
194-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
195-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
204+
let mut features = NodeFeatures::empty();
205+
features.set_onion_messages_optional();
206+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
207+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
196208

197209
let mut path = super::find_path(&our_id, &node_pks[5], &network_graph, None, Arc::clone(&logger)).unwrap();
198210
assert_eq!(path.len(), 3);
@@ -203,10 +215,32 @@ mod tests {
203215

204216
#[test]
205217
fn long_path() {
206-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_line_graph();
207-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
218+
let mut features = NodeFeatures::empty();
219+
features.set_onion_messages_optional();
220+
let (secp_ctx, network_graph, _, _, logger) = build_line_graph(features);
221+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
208222

209223
let path = super::find_path(&our_id, &node_pks[18], &network_graph, None, Arc::clone(&logger)).unwrap();
210224
assert_eq!(path.len(), 19);
211225
}
226+
227+
#[test]
228+
fn disable_nodes_test() {
229+
// Check that we won't route over nodes that require unknown feature bits.
230+
let mut features = InitFeatures::empty();
231+
features.set_onion_messages_optional();
232+
let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph_with_features(features.to_context());
233+
let (_, our_id, privkeys, node_pks) = get_nodes(&secp_ctx);
234+
235+
// Disable nodes 1, 2, and 8 by requiring unknown feature bits
236+
let mut unknown_features = NodeFeatures::empty();
237+
unknown_features.set_unknown_feature_required();
238+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[0], unknown_features.clone(), 1);
239+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[1], unknown_features.clone(), 1);
240+
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[7], unknown_features.clone(), 1);
241+
242+
// If all nodes require some features we don't understand, route should fail
243+
let err = super::find_path(&our_id, &node_pks[2], &network_graph, None, Arc::clone(&logger)).unwrap_err();
244+
assert_eq!(err, super::Error::PathNotFound);
245+
}
212246
}

‎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)