Skip to content

Report the sending peer in Event::OnionMessageIntercepted#4682

Open
jkczyz wants to merge 2 commits into
lightningdevkit:mainfrom
jkczyz:2026-06-onion-message-origin
Open

Report the sending peer in Event::OnionMessageIntercepted#4682
jkczyz wants to merge 2 commits into
lightningdevkit:mainfrom
jkczyz:2026-06-onion-message-origin

Conversation

@jkczyz

@jkczyz jkczyz commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

When the OnionMessenger intercepts a message bound for an offline peer, it now reports which peer sent us the message to forward via a new prev_node_id field, so handlers can apply source-based policy when deciding whether to forward. The existing destination field is renamed peer_node_id -> next_node_id so the two node ids are unambiguous.

prev_node_id is None when the forward is enqueued by a message handler (the BOLT 12 static-invoice-server flow), which isn't given the sending node; otherwise it is the node we received the message from.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 11, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@jkczyz jkczyz requested a review from TheBlueMatt June 11, 2026 14:28
@ldk-claude-review-bot

ldk-claude-review-bot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Found one issue.

lightning/src/onion_message/messenger.rs:1739 — In the offline-peer interception path, the event reuses next_hop verbatim. When the next hop was a ShortChannelId that successfully resolved to next_node_id (a known-but-offline peer), the emitted event carries a ShortChannelId next hop even though intercept_for_unknown_scids is disabled. This contradicts the documented invariant (events/mod.rs:1844), breaks the documented LDK 0.2 downgrade safety (the PR's own test_onion_message_intercepted_scid_downgrade_to_0_2 proves 0.2 cannot decode a ShortChannelId variant), and deprives the handler of the resolved node id it needs to wake the peer. Should report NextMessageHop::NodeId(next_node_id) instead.

The serialization/TLV compatibility logic (field 0 legacy peer_node_id, field 1 next_hop, field 3 prev_hop, with the read-side .or() fallback) is otherwise correct and the call sites are consistent.

@TheBlueMatt TheBlueMatt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks. hate changing the api at this point but backporting to 0.3 probably should happen cause its trivial

Comment thread lightning/src/events/mod.rs Outdated
@ldk-reviews-bot

Copy link
Copy Markdown

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@jkczyz jkczyz force-pushed the 2026-06-onion-message-origin branch from e0c7e55 to 41ef2e6 Compare June 11, 2026 15:01
@jkczyz jkczyz requested a review from TheBlueMatt June 11, 2026 15:01

@tnull tnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, this seems to conflict with the first commit on #4463. Do you maybe want to pull that commit here to do it in one go?

@jkczyz jkczyz self-assigned this Jun 11, 2026
tnull and others added 2 commits June 11, 2026 13:39
Allow integrations to intercept blinded onion-message hops that identify
the next node by short channel id, so LSPS-style protocols can resolve
those hops out of band instead of dropping the message.

Co-Authored-By: HAL 9000
When the OnionMessenger intercepts an onion message to forward, it now reports
which peer sent us the message via a new `prev_hop` field, so handlers can
apply source-based policy when deciding whether to forward.

`prev_hop` is `None` when the forward is enqueued by a message handler (the
BOLT 12 static-invoice-server flow), which isn't given the sending node;
otherwise it is the node we received the message from.

Co-Authored-By: Claude <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-06-onion-message-origin branch from 41ef2e6 to 316a0ce Compare June 11, 2026 19:12
@jkczyz

jkczyz commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Hmm, this seems to conflict with the first commit on #4463. Do you maybe want to pull that commit here to do it in one go?

Good idea! Done, but needed to rebase.

@jkczyz jkczyz requested a review from tnull June 11, 2026 19:13
Comment on lines 1739 to 1743
self.enqueue_intercepted_event(Event::OnionMessageIntercepted {
peer_node_id: next_node_id,
prev_hop,
next_hop,
message: onion_message,
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the offline-peer interception path the event reuses next_hop verbatim. But this branch is reached even when next_hop was a ShortChannelId that was successfully resolved to next_node_id by node_id_lookup (line 1681) — i.e. a known-but-offline peer. In that case the emitted event carries next_hop: ShortChannelId(..) despite intercept_for_unknown_scids being disabled.

This has three consequences:

  1. It contradicts the doc on OnionMessageIntercepted ("The ShortChannelId variant is only generated if intercept_for_unknown_scids was set").
  2. It breaks the documented LDK 0.2 downgrade safety — test_onion_message_intercepted_scid_downgrade_to_0_2 shows 0.2 cannot decode a ShortChannelId variant, yet this path can produce one for users who only enabled offline-peer interception.
  3. The handler is supposed to wake the offline peer, but it now receives an SCID instead of the next_node_id we already resolved.

This branch should report the resolved node id:

Suggested change
self.enqueue_intercepted_event(Event::OnionMessageIntercepted {
peer_node_id: next_node_id,
prev_hop,
next_hop,
message: onion_message,
});
self.enqueue_intercepted_event(Event::OnionMessageIntercepted {
prev_hop,
next_hop: NextMessageHop::NodeId(next_node_id),
message: onion_message,
});
Ok(())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants