Skip to content

Commit 3e7e411

Browse files
valentinewallacevalentinewallace
committed
OM pathfinding: don't consider nodes that don't support OM forwarding
Or that require unknown features
1 parent 9b87261 commit 3e7e411

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
@@ -60,6 +60,11 @@ pub fn find_path<L: Deref, GL: Deref>(
6060
return Ok(reverse_path(visited, our_node_id, dest_node_id, logger)?)
6161
}
6262
if let Some(node_info) = network_nodes.get(&node_id) {
63+
if node_id == our_node_id {
64+
} else if let Some(node_ann) = &node_info.announcement_info {
65+
if !node_ann.features.supports_onion_messages() || node_ann.features.requires_unknown_bits()
66+
{ continue; }
67+
}
6368
for scid in &node_info.channels {
6469
if let Some(chan_info) = network_channels.get(&scid) {
6570
if let Some((_, successor)) = chan_info.as_directed_from(&node_id) {
@@ -136,14 +141,17 @@ fn reverse_path<L: Deref>(
136141

137142
#[cfg(test)]
138143
mod tests {
139-
use routing::test_utils;
144+
use ln::features::{InitFeatures, NodeFeatures};
145+
use routing::test_utils::{add_or_update_node, build_graph_with_features, build_line_graph, get_nodes};
140146

141147
use sync::Arc;
142148

143149
#[test]
144150
fn one_hop() {
145-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
146-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
151+
let mut features = NodeFeatures::empty();
152+
features.set_onion_messages_optional();
153+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
154+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
147155

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

153161
#[test]
154162
fn two_hops() {
155-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
156-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
163+
let mut features = NodeFeatures::empty();
164+
features.set_onion_messages_optional();
165+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
166+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
157167

158168
let path = super::find_path(&our_id, &node_pks[2], &network_graph, None, Arc::clone(&logger)).unwrap();
159169
assert_eq!(path.len(), 2);
@@ -164,8 +174,10 @@ mod tests {
164174

165175
#[test]
166176
fn three_hops() {
167-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_graph();
168-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
177+
let mut features = NodeFeatures::empty();
178+
features.set_onion_messages_optional();
179+
let (secp_ctx, network_graph, _, _, logger) = build_graph_with_features(features);
180+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
169181

170182
let mut path = super::find_path(&our_id, &node_pks[5], &network_graph, None, Arc::clone(&logger)).unwrap();
171183
assert_eq!(path.len(), 3);
@@ -176,12 +188,34 @@ mod tests {
176188

177189
#[test]
178190
fn long_path() {
179-
let (secp_ctx, network_graph, _, _, logger) = test_utils::build_line_graph();
180-
let (_, our_id, _, node_pks) = test_utils::get_nodes(&secp_ctx);
191+
let mut features = NodeFeatures::empty();
192+
features.set_onion_messages_optional();
193+
let (secp_ctx, network_graph, _, _, logger) = build_line_graph(features);
194+
let (_, our_id, _, node_pks) = get_nodes(&secp_ctx);
181195

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

187221
#[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)