Skip to content

Stop using an introduction node in blinded message paths #4647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TheBlueMatt wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −191
Open

Stop using an introduction node in blinded message paths #4647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 2 additions & 155 deletions lightning/src/ln/offers_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use crate::ln::channelmanager::{PaymentId, RecentPaymentDetails, self};
use crate::ln::outbound_payment::{Bolt12PaymentError, RecipientOnionFields, Retry};
use crate::types::features::Bolt12InvoiceFeatures;
use crate::ln::functional_test_utils::*;
use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, Init, NodeAnnouncement, OnionMessage, OnionMessageHandler, RoutingMessageHandler, SocketAddress, UnsignedGossipMessage, UnsignedNodeAnnouncement};
use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, Init, OnionMessage, OnionMessageHandler};
use crate::ln::outbound_payment::IDEMPOTENCY_TIMEOUT_TICKS;
use crate::offers::invoice::Bolt12Invoice;
use crate::offers::invoice_error::InvoiceError;
Expand All @@ -66,9 +66,8 @@ use crate::offers::offer::OfferBuilder;
use crate::offers::parse::Bolt12SemanticError;
use crate::onion_message::messenger::{DefaultMessageRouter, Destination, MessageRouter, MessageSendInstructions, NodeIdMessageRouter, NullMessageRouter, PeeledOnion, DUMMY_HOPS_PATH_LENGTH, QR_CODED_DUMMY_HOPS_PATH_LENGTH};
use crate::onion_message::offers::OffersMessage;
use crate::routing::gossip::{NodeAlias, NodeId};
use crate::routing::router::{DEFAULT_PAYMENT_DUMMY_HOPS, PaymentParameters, RouteParameters, RouteParametersConfig};
use crate::sign::{NodeSigner, Recipient};
use crate::sign::NodeSigner;
use crate::util::ser::Writeable;

/// This used to determine whether we built a compact path or not, but now its just a random
Expand Down Expand Up @@ -125,38 +124,6 @@ fn disconnect_peers<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, peers: &[&Node<'a, 'b
}
}

fn announce_node_address<'a, 'b, 'c>(
node: &Node<'a, 'b, 'c>, peers: &[&Node<'a, 'b, 'c>], address: SocketAddress,
) {
let features = node.onion_messenger.provided_node_features()
| node.gossip_sync.provided_node_features();
let rgb = [0u8; 3];
let announcement = UnsignedNodeAnnouncement {
features,
timestamp: 1000,
node_id: NodeId::from_pubkey(&node.keys_manager.get_node_id(Recipient::Node).unwrap()),
rgb,
alias: NodeAlias([0u8; 32]),
addresses: vec![address],
excess_address_data: Vec::new(),
excess_data: Vec::new(),
};
let signature = node.keys_manager.sign_gossip_message(
UnsignedGossipMessage::NodeAnnouncement(&announcement)
).unwrap();

let msg = NodeAnnouncement {
signature,
contents: announcement
};

let node_pubkey = node.node.get_our_node_id();
node.gossip_sync.handle_node_announcement(None, &msg).unwrap();
for peer in peers {
peer.gossip_sync.handle_node_announcement(Some(node_pubkey), &msg).unwrap();
}
}

fn resolve_introduction_node<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, path: &BlindedMessagePath) -> PublicKey {
path.public_introduction_node_id(&node.network_graph.read_only())
.and_then(|node_id| node_id.as_pubkey().ok())
Expand Down Expand Up @@ -362,126 +329,6 @@ fn create_refund_with_no_blinded_path() {
assert!(refund.paths().is_empty());
}

/// Checks that blinded paths without Tor-only nodes are preferred when constructing an offer.
#[test]
fn prefers_non_tor_nodes_in_blinded_paths() {
let mut accept_forward_cfg = test_default_channel_config();
accept_forward_cfg.accept_forwards_to_priv_channels = true;

let mut features = channelmanager::provided_init_features(&accept_forward_cfg);
features.set_onion_messages_optional();
features.set_route_blinding_optional();

let chanmon_cfgs = create_chanmon_cfgs(6);
let node_cfgs = create_node_cfgs(6, &chanmon_cfgs);

*node_cfgs[1].override_init_features.borrow_mut() = Some(features);

let node_chanmgrs = create_node_chanmgrs(
6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None]
);
let nodes = create_network(6, &node_cfgs, &node_chanmgrs);

create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000);
create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000);

// Add an extra channel so that more than one of Bob's peers have MIN_PEER_CHANNELS.
create_announced_chan_between_nodes_with_value(&nodes, 4, 5, 10_000_000, 1_000_000_000);

let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]);
let bob_id = bob.node.get_our_node_id();
let charlie_id = charlie.node.get_our_node_id();

disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]);
disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]);

let tor = SocketAddress::OnionV2([255, 254, 253, 252, 251, 250, 249, 248, 247, 246, 38, 7]);
announce_node_address(charlie, &[alice, bob, david, &nodes[4], &nodes[5]], tor.clone());

let offer = bob.node
.create_offer_builder().unwrap()
.amount_msats(10_000_000)
.build().unwrap();
assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id));
assert!(!offer.paths().is_empty());
for path in offer.paths() {
let introduction_node_id = resolve_introduction_node(david, &path);
assert_ne!(introduction_node_id, bob_id);
assert_ne!(introduction_node_id, charlie_id);
}

// Use a one-hop blinded path when Bob is announced and all his peers are Tor-only.
announce_node_address(&nodes[4], &[alice, bob, charlie, david, &nodes[5]], tor.clone());
announce_node_address(&nodes[5], &[alice, bob, charlie, david, &nodes[4]], tor.clone());

let offer = bob.node
.create_offer_builder().unwrap()
.amount_msats(10_000_000)
.build().unwrap();
assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id));
assert!(!offer.paths().is_empty());
for path in offer.paths() {
let introduction_node_id = resolve_introduction_node(david, &path);
assert_eq!(introduction_node_id, bob_id);
}
}

/// Checks that blinded paths prefer an introduction node that is the most connected.
#[test]
fn prefers_more_connected_nodes_in_blinded_paths() {
let mut accept_forward_cfg = test_default_channel_config();
accept_forward_cfg.accept_forwards_to_priv_channels = true;

let mut features = channelmanager::provided_init_features(&accept_forward_cfg);
features.set_onion_messages_optional();
features.set_route_blinding_optional();

let chanmon_cfgs = create_chanmon_cfgs(6);
let node_cfgs = create_node_cfgs(6, &chanmon_cfgs);

*node_cfgs[1].override_init_features.borrow_mut() = Some(features);

let node_chanmgrs = create_node_chanmgrs(
6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None]
);
let nodes = create_network(6, &node_cfgs, &node_chanmgrs);

create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000);
create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000);

// Add extra channels so that more than one of Bob's peers have MIN_PEER_CHANNELS and one has
// more than the others.
create_announced_chan_between_nodes_with_value(&nodes, 0, 4, 10_000_000, 1_000_000_000);
create_announced_chan_between_nodes_with_value(&nodes, 3, 4, 10_000_000, 1_000_000_000);

let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]);
let bob_id = bob.node.get_our_node_id();

disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]);
disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]);

let offer = bob.node
.create_offer_builder().unwrap()
.amount_msats(10_000_000)
.build().unwrap();
assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id));
assert!(!offer.paths().is_empty());
for path in offer.paths() {
let introduction_node_id = resolve_introduction_node(david, &path);
assert_eq!(introduction_node_id, nodes[4].node.get_our_node_id());
}
}

/// Tests the dummy hop behavior of Offers based on the message router used:
/// - Compact paths (`DefaultMessageRouter`) should not include dummy hops.
/// - Node ID paths (`NodeIdMessageRouter`) may include 0 to [`MAX_DUMMY_HOPS_COUNT`] dummy hops.
Expand Down
69 changes: 33 additions & 36 deletions lightning/src/onion_message/messenger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,6 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Logger, ES: EntropySource>
// Limit the number of blinded paths that are computed.
const MAX_PATHS: usize = 3;

// Ensure peers have at least three channels so that it is more difficult to infer the
// recipient's node_id.
const MIN_PEER_CHANNELS: usize = 3;

let network_graph = network_graph.deref().read_only();
let is_recipient_announced =
network_graph.nodes().contains_key(&NodeId::from_pubkey(&recipient));
Expand Down Expand Up @@ -596,32 +592,6 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Logger, ES: EntropySource>

let compact_paths = !never_compact_path && size_constrained;

let has_one_peer = peers.len() == 1;
let mut peer_info = peers
.map(|peer| MessageForwardNode {
short_channel_id: if compact_paths { peer.short_channel_id } else { None },
..peer
})
// Limit to peers with announced channels unless the recipient is unannounced.
.filter_map(|peer| {
network_graph
.node(&NodeId::from_pubkey(&peer.node_id))
.filter(|info| {
!is_recipient_announced || info.channels.len() >= MIN_PEER_CHANNELS
})
.map(|info| (peer, info.is_tor_only(), info.channels.len()))
// Allow messages directly with the only peer when unannounced.
.or_else(|| (!is_recipient_announced && has_one_peer).then(|| (peer, false, 0)))
})
// Exclude Tor-only nodes when the recipient is announced.
.filter(|(_, is_tor_only, _)| !(*is_tor_only && is_recipient_announced))
.collect::<Vec<_>>();

// Prefer using non-Tor nodes with the most channels as the introduction node.
peer_info.sort_unstable_by(|(_, a_tor_only, a_channels), (_, b_tor_only, b_channels)| {
a_tor_only.cmp(b_tor_only).then(a_channels.cmp(b_channels).reverse())
});

let build_path = |intermediate_hops: &[MessageForwardNode]| {
// Calculate the dummy hops given the total hop count target (including the recipient).
let dummy_hops_count = path_len_incl_dummys.saturating_sub(intermediate_hops.len() + 1);
Expand All @@ -638,12 +608,39 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Logger, ES: EntropySource>
)
};

// Try to create paths from peer info, fall back to direct path if needed
let mut paths = peer_info
.into_iter()
.map(|(peer, _, _)| build_path(&[peer]))
.take(MAX_PATHS)
.collect::<Vec<_>>();
let has_one_peer = peers.len() == 1;
let mut paths = if !is_recipient_announced {
let mut peer_info = peers
.map(|peer| MessageForwardNode {
short_channel_id: if compact_paths { peer.short_channel_id } else { None },
..peer
})
.filter_map(|peer| {
network_graph
.node(&NodeId::from_pubkey(&peer.node_id))
.map(|info| (peer, info.is_tor_only(), info.channels.len()))
// Allow messages directly with the only peer
.or_else(|| has_one_peer.then(|| (peer, false, 0)))
})
// Exclude Tor-only nodes when the recipient is announced.
.filter(|(_, is_tor_only, _)| !(*is_tor_only && is_recipient_announced))
Comment on lines +625 to +626
Copy link
Copy Markdown
Collaborator

@ldk-claude-review-bot ldk-claude-review-bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This filter and its comment are dead code. Since we're inside the if !is_recipient_announced branch (line 612), is_recipient_announced is always false here, so !(*is_tor_only && false) is always true — the filter never excludes anything.

The comment "Exclude Tor-only nodes when the recipient is announced" is also misleading since this branch is specifically for unannounced recipients.

Either remove the filter entirely (since it's a no-op), or if the intent is to exclude Tor-only introduction nodes for unannounced recipients too, change it to .filter(|(_, is_tor_only, _)| !*is_tor_only).

.collect::<Vec<_>>();

// Prefer using non-Tor nodes with the most channels as the introduction node.
peer_info.sort_unstable_by(|(_, a_tor_only, a_channels), (_, b_tor_only, b_channels)| {
a_tor_only.cmp(b_tor_only).then(a_channels.cmp(b_channels).reverse())
Copy link
Copy Markdown
Collaborator

@ldk-claude-review-bot ldk-claude-review-bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the Tor-only component of this sort is now effectively dead — since the Tor-only filter above is a no-op (see comment there), Tor-only nodes do still get sorted to the back, but they're never excluded. If excluding Tor-only nodes for unannounced recipients isn't intended, the is_tor_only tracking and sorting could be cleaned up for clarity (just sort by channel count).

});

// Try to create paths from peer info, fall back to direct path if needed
peer_info
.into_iter()
.map(|(peer, _, _)| build_path(&[peer]))
.take(MAX_PATHS)
.collect::<Vec<_>>()
} else {
vec![]
};

if paths.is_empty() {
if is_recipient_announced {
paths = vec![build_path(&[])];
Expand Down
Loading