Static invoice server: treat forwarded invoice requests as onion message forwards #4114

valentinewallace · 2025-09-23T21:28:33Z

Previously, when a static invoice server forwarded an invoice request to an
often-offline recipient, they would treat the outbound message like any other
outbound onion message initiated by their own node. That means they would
buffer the onion message internally in the onion messenger and generate a
ConnectionNeeded event if the next-hop node was offline.

Buffering the onion message in this case poses a DoS risk for the invoice
server node, since they do not control the quantity of invoice requests they
receive on behalf of often-offline recipients. Instead, we should treat these
forwarded invoice requests like any other onion message that needs to be
forwarded -- if the next-hop node is offline, either drop the message or
generate an OnionMessageIntercepted event for it (pushing the DoS management
onto the handler of the interception event).

Closes #4110

Also closes #4112:

Imagine an LSP with a mobile client. The LSP wants to pay an offer that client
issued. It generates an onion messages invreq and sends it. The OnionMessenger
notices it's not connected and searches the network graph. It doesn't find the
mobile client so doesn't create a direct connect event. The message gets
dropped and the payment fails.

Here we start generating the DC event irrespective of the network graph so the
LSP can use LSPS5 to wake the client.

ldk-reviews-bot · 2025-09-23T21:28:36Z

👋 Thanks for assigning @TheBlueMatt 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.

graphite-app · 2025-09-23T21:29:53Z

lightning/src/onion_message/messenger.rs

+			NextMessageHop::ShortChannelId(scid) => match self.node_id_lookup.next_node_id(scid) {
+				Some(pubkey) => pubkey,
+				None => {
+					log_trace!(self.logger, "Dropping forwarded onion messager: unable to resolve next hop using SCID {} {}", scid, log_suffix);


There's a typo in the log message: messager should be message to maintain consistency with other error messages in this file.

Suggested change

log_trace!(self.logger, "Dropping forwarded onion messager: unable to resolve next hop using SCID {} {}", scid, log_suffix);

log_trace!(self.logger, "Dropping forwarded onion message: unable to resolve next hop using SCID {} {}", scid, log_suffix);

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

codecov · 2025-09-23T21:43:06Z

Codecov Report

❌ Patch coverage is 86.76471% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.53%. Comparing base (f707245) to head (1bd3f36).
⚠️ Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/onion_message/messenger.rs	85.04%	14 Missing and 2 partials ⚠️
lightning/src/ln/async_payments_tests.rs	85.71%	0 Missing and 1 partial ⚠️
lightning/src/onion_message/functional_tests.rs	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4114      +/-   ##
==========================================
- Coverage   88.72%   88.53%   -0.19%     
==========================================
  Files         177      179       +2     
  Lines      133286   134333    +1047     
  Branches   133286   134333    +1047     
==========================================
+ Hits       118253   118938     +685     
- Misses      12326    12638     +312     
- Partials     2707     2757      +50

Flag	Coverage Δ
fuzzing	`21.79% <38.53%> (+0.05%)`	⬆️
tests	`88.37% <86.46%> (-0.19%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt

A few small comments, but mostly looks good.

lightning/src/events/mod.rs

TheBlueMatt · 2025-09-23T22:28:05Z

lightning/src/events/mod.rs

 		/// recipient is online to provide a new invoice. This path should be persisted and
 		/// later provided to [`ChannelManager::respond_to_static_invoice_request`].
 		///
+		/// This path's [`BlindedMessagePath::introduction_node`] MUST be set to our node or one of our


Shouldn't this be documented on the client end of the protocol not the server end?

I generally agree, though the only other possible place I see is on the ServeStaticInvoice onion message field where no one will see it. Maybe that's fine? There's nothing clients can do in practice to satisfy this requirement right now anyway since LDK generates the path without their intervention, so I guess I'll move it.

Edit: basically duplicated the docs in both places so they are more likely to be surfaced

lightning/src/onion_message/messenger.rs

ldk-reviews-bot · 2025-09-23T22:30:53Z

👋 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.

In an upcoming commit, we will need to repurpose this logic when, as a static invoice server node, we update our support for forwarding invoice requests from payers to often-offline recipients. We currently treat these forwarded invreqs as outbound onion messages that are initiated by our node, when they should really be treated as forwarded onion messages.

Makes the next commit cleaner when we add support for forwarding onion messages within send_onion_message_internal. Also rename enqueue_onion_message to specify that it is used for outbound onion messages.

Previously, when a static invoice server forwarded an invoice request to an often-offline recipient, they would treat the outbound message like any other outbound onion message initiated by their own node. That means they would buffer the onion message internally in the onion messenger and generate a ConnectionNeeded event if the next-hop node was offline. Buffering the onion message in this case poses a DoS risk for the invoice server node, since they do not control the quantity of invoice requests they receive on behalf of often-offline recipients. Instead, we should treat these forwarded invoice requests like any other onion message that needs to be forwarded -- if the next-hop node is offline, either drop the message or generate an OnionMessageIntercepted event for it (pushing the DoS management onto the handler of the interception event).

valentinewallace · 2025-09-24T17:52:37Z

lightning/src/onion_message/messenger.rs

+				Some((features, addresses)) if features.supports_onion_messages() => {
 					Ok(OnionMessagePath {
 						intermediate_nodes: vec![],
 						destination,
-						first_node_addresses,
+						first_node_addresses: addresses.to_vec(),
+					})
+				},


Maybe we should not do this new always-generate-DC-event behavior if the node is announced, actually?

You mean announced && !supports_onion_messages? That seems reasonable. Otherwise we could even drop the supports_onion_messages check now, seems weird to generate the DC event for a peer that doesn't support OMs and just drop the connection addresses lol.

I didn't follow that -- how can we drop the OM feature check but also avoid generating DC events for non-OM-supporting peers? I pushed what I think is correct.

lightning/src/onion_message/messenger.rs

TheBlueMatt

Two comments, otherwise LGTM.

Imagine an LSP with a mobile client. The LSP wants to pay an offer that client issued. It generates an onion messages invreq and sends it. The OnionMessenger notices it's not connected and searches the network graph. It doesn't find the mobile client so doesn't create a direct connect event. The message gets dropped and the payment fails. Here we start generating the DC event irrespective of the network graph so the LSP can use LSPS5 to wake the client.

TheBlueMatt

Just gonna land this. Its quite straightforward.

valentinewallace added this to the 0.2 milestone Sep 23, 2025

valentinewallace requested a review from TheBlueMatt September 23, 2025 21:28

graphite-app bot reviewed Sep 23, 2025

View reviewed changes

TheBlueMatt reviewed Sep 23, 2025

View reviewed changes

valentinewallace added 3 commits September 24, 2025 12:42

Construct outbound OMs before enqueueing them

332b3e5

Makes the next commit cleaner when we add support for forwarding onion messages within send_onion_message_internal. Also rename enqueue_onion_message to specify that it is used for outbound onion messages.

valentinewallace force-pushed the 2025-09-fwd-invreq-in-mailbox branch from 9d3a2bc to 2460c46 Compare September 24, 2025 16:44

valentinewallace requested a review from TheBlueMatt September 24, 2025 16:49

valentinewallace force-pushed the 2025-09-fwd-invreq-in-mailbox branch from 8c9ceef to e0a805f Compare September 24, 2025 17:49

valentinewallace commented Sep 24, 2025

View reviewed changes

valentinewallace force-pushed the 2025-09-fwd-invreq-in-mailbox branch from e0a805f to 9e2b626 Compare September 24, 2025 18:47

TheBlueMatt reviewed Sep 24, 2025

View reviewed changes

lightning/src/onion_message/messenger.rs Show resolved Hide resolved

TheBlueMatt reviewed Sep 24, 2025

View reviewed changes

valentinewallace force-pushed the 2025-09-fwd-invreq-in-mailbox branch from 9e2b626 to b03f377 Compare September 24, 2025 20:47

valentinewallace force-pushed the 2025-09-fwd-invreq-in-mailbox branch from b03f377 to 1bd3f36 Compare September 24, 2025 20:48

TheBlueMatt approved these changes Sep 24, 2025

View reviewed changes

TheBlueMatt merged commit 7926fb9 into lightningdevkit:main Sep 24, 2025
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Static invoice server: treat forwarded invoice requests as onion message forwards #4114

Static invoice server: treat forwarded invoice requests as onion message forwards #4114

Uh oh!

valentinewallace commented Sep 23, 2025 •

edited

Loading

Uh oh!

ldk-reviews-bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

graphite-app bot Sep 23, 2025

Uh oh!

codecov bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

TheBlueMatt left a comment

Uh oh!

Uh oh!

TheBlueMatt Sep 23, 2025

Uh oh!

valentinewallace Sep 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented Sep 23, 2025

Uh oh!

valentinewallace Sep 24, 2025

Uh oh!

TheBlueMatt Sep 24, 2025

Uh oh!

valentinewallace Sep 24, 2025

Uh oh!

Uh oh!

TheBlueMatt left a comment

Uh oh!

TheBlueMatt left a comment

Uh oh!

Uh oh!

Uh oh!

	log_trace!(self.logger, "Dropping forwarded onion messager: unable to resolve next hop using SCID {} {}", scid, log_suffix);
	log_trace!(self.logger, "Dropping forwarded onion message: unable to resolve next hop using SCID {} {}", scid, log_suffix);

Static invoice server: treat forwarded invoice requests as onion message forwards #4114

Static invoice server: treat forwarded invoice requests as onion message forwards #4114

Uh oh!

Conversation

valentinewallace commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

valentinewallace Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented Sep 23, 2025

Uh oh!

valentinewallace Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

valentinewallace Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

valentinewallace commented Sep 23, 2025 •

edited

Loading

ldk-reviews-bot commented Sep 23, 2025 •

edited

Loading

codecov bot commented Sep 23, 2025 •

edited

Loading

valentinewallace Sep 24, 2025 •

edited

Loading